Milab-6720: import vdj sequences - #47
Merged
Merged
Conversation
Automated output of `pnpm run upgrade-sdk` (block-tools structure refresh). No hand edits in this commit. - SDK catalog: model/ui 1.79.14 -> 1.81.1, workflow-tengo 6.6.3 -> 6.8.2, block-tools 2.11.0 -> 2.13.0, test 1.79.14 -> 1.81.3, tengo-builder 4.0.8 -> 4.0.22, package-builder 3.13.0 -> 3.15.0, ts-builder 1.5.2 -> 1.6.2, ts-configs 1.2.3 -> 1.4.0 - block/ reshaped into the slim facade: index.d.ts/index.js replaced by src/ (index.ts, block-extra.ts, AGENTS.ts, agents-extra.ts) + tsconfig - root build scripts replaced: build/build:dev -> build:dev-local, build:dev-remote, build:dev-no-software, build:dev-binary-existing, build:release; PL_PKG_DEV -> PL_BUILD_CHANNEL/VARIANT/LOCATION - CI: build-script-name build -> build:dev-local, adds build-before-publish-script-name build:release, require-package-path-bump, hz-ubuntu-dind runner
Four fixes the structure refresh could not make itself. - model: createPlDataTableV2 now requires PColumn<PColumnDataUniversal>[], while PColumnCollection.getColumns() is typed with data possibly undefined. Narrow with a type guard. Runtime behaviour unchanged: without `dontWaitAllData` getColumns returns undefined for the whole request when any data is incomplete, so a returned array already has data everywhere. - model/ui: export the model as `platforma`, not `model`. The generated block facade (block/src/index.ts, structurer-owned) imports that name, and it is the name every V3 block uses. Renamed rather than aliased to avoid two names for one value. - catalog: pin @milaboratories/helpers to 1.14.5 to match what @platforma-sdk/model 1.81.1 depends on. With 1.14.2 two copies resolved and the model's inferred type referenced the non-direct one, failing TS2742. - package.json: restore packageManager pnpm@9.14.4. The refresh reverted it to 9.12.0, undoing c93ad7e ("Set pnpm to Platforma version") and leaving root disagreeing with block/package.json, which still said 9.14.4.
Adds the region-annotation capability as a standalone, callable unit. Nothing in the import pipeline invokes it yet — that lands with the bare-set emission path. software/region-annotation/ — python, two entrypoints: fasta.py one FASTA record per (key, chain) that carries a sequence main.py projects ANARCI numbering onto region ranges, emits regions + status regions.py holds the pure logic and the per-scheme range table workflow/src/annotate-regions.tpl.tengo — fasta -> ANARCI --scheme -> regions. Four things differ deliberately from redefine-clonotypes' anarci-numbering, which this is forked from: - All seven regions, FR4 included. The three region lists in the workspace are each short in a different way and FR4 is the one missing everywhere. - A per-chain status column with a closed three-member enum, never null. The block this is forked from gives an unnumbered record empty region strings, which reads downstream as a located region that happens to be empty. - Chain vocabulary is A/B (declared by the mapping slot), not H/KL (inferred by ANARCI). Lookup keys on the full "key|chain" FASTA id across both buckets, so the two chains of one paired record cannot collide, and a record whose declared chain disagrees with ANARCI's bucket is annotated under the declared chain using the bucket's range table. Reconciling declared against inferred is out of scope. - Amino acid only; a bare set carries no nucleotide variable domain. Both ANARCI bucket CSVs are pre-created with an Id header before the run so the exec can always save both, following the same trick in antibody-tcr-lead-selection/workflow/src/assembling-fasta.tpl.tengo. Verified by software/region-annotation/test_region_annotation.py against synthetic ANARCI CSVs: region arithmetic per bucket, all four status paths, single-chain sets, and header-only buckets. Not in CI — it needs python with polars, which the node test lane does not provide.
The chain a column is emitted under is declared by the mapping slot; ANARCI infers H/KL from the sequence itself. Those can disagree, and the annotation is correct either way — the record is labelled with its declared chain and its boundaries are read in the coordinate space of the bucket ANARCI actually used. What that leaves unguarded is the likeliest mapping mistake there is. Map the two sequence columns to the wrong slots and the import succeeds completely: every record Annotated, every value internally consistent, every chain label wrong, and Sequence Liabilities then scores heavy-chain positional rules against light chains. Nothing else in the output reveals it, because with chain fixed by the slot there is nothing to cross-check it against. So the numbering step now emits per-chain outcome counts, including `chainDisagreed`: annotated records whose bucket was not the one their declared chain implies. Reported through a new `stats` output; nothing acts on it. Counting is not reconciling. The spec defers reconciliation and the threshold at which disagreement should stop a run, on the stated grounds that the rate has never been measured. This is that measurement, not the decision it feeds. A few disagreements are odd rows; nearly all of them on both chains are swapped columns, and the number tells those apart on its own. Also untracks two .pyc files that slipped into the previous commit and ignores __pycache__. A stale .pyc inside a dev block archive shadows the edited source, so the backend keeps running the old code with no sign of it.
The axis and column specs a bare set is emitted on, in one library, with the atom that fixes each one named above it. This is transcription rather than design — every domain key, annotation and order priority is settled in the spec corpus. The shape worth stating once: every property column sits on [pl7.app/sampleId, pl7.app/variantKey], and there is ONE frame, not one per chain. The two chains of a paired record are separated by the scClonotypeChain column domain, never by an axis and never by a separate frame. That is the whole difference from the per-chain path the other seven formats take. Nothing emits these yet; the import path that consumes them lands next. Three judgement calls inside the transcription, all noted in the source: - Labels render Heavy/Light while domains stay A/B. The atoms write <chain> in both positions, but the one shipped paired producer in the workspace does it this way (mixcr-scfv-clonotyping/workflow/src/mixcr-exports.lib.tengo:173-178). A literal reading would put "A CDR3 aa" in front of the scientist. - The label column's own spec is not fixed by any atom — the atom settles the value, not the spec. It mirrors the block's existing clonotypeLabel column. - The status column carries no scClonotypeChain/index key while the sequence columns do. That is the atom as written, and it reads correctly: the status is a property of the chain's annotation attempt, not of one of its sequences. 23 tengo unit tests cover it, and the workflow package now runs pl-tengo test, so they execute in CI. They pin the things that are silent when wrong rather than the things that throw: that the run-id key claims VDJ and not peptide or amplicon; that the two chains produce distinct specs, since identical ones would dedupe into a single column; that the convention appears in BOTH the domain and the annotation, because dropping the annotation still computes liabilities and computes them against the wrong positions; that exactly one column claims to be the anchor and primary abundance; that the synthetic abundance carries no unit and no synthetic marker; that sampleCount and the cross-sample aggregates are absent; and that assembly is canonical, since map iteration there would break downstream dedup with no error.
Wires the emission path end to end: mapping -> unpivot -> key -> ANARCI ->
one frame. Builds and type-checks; it has not yet been run against a backend,
so nothing here is verified behaviour.
The path is selected by the presence of a chain-slotted sequence mapping and an
identity column, not by a format the scientist picks. Whether a set is bare is
something the block works out from what was mapped — asking up front was the
rejected alternative, and the UI accordingly offers the slots rather than a mode.
Structure:
import-bare-set.tpl.tengo per sample: unpivot, mint the key, presence=1,
then annotate. No V/J filter, no locus matching,
no groupBy collapse.
process-bare-set.tpl.tengo one processColumn pass into one frame.
annotate-regions.lib.tengo was a template; now a library, because its only
caller is a processColumn body, and a tpl.light
body runs execs but does not render templates.
bare-set-specs.lib.tengo gains columnsBySource, so the process template
does not infer from column names which TSV a
column comes from.
One pass, not two. The per-chain path takes a second processColumn only to
compute sampleCount and the CountTotal/FractionMean pair across samples, and a
bare set emits none of those. Since every emitted column sits on
[sampleId, variantKey] with nothing collapsing across samples, the second pass
has no work left. The suppression is therefore structural rather than a filter:
the pass that computed those columns is absent.
Model: bareSet is additive to BlockArgs, so no persisted state changes shape and
no existing project needs a migration. It is also the one field V3 will re-home,
which keeps that migration mechanical.
First end-to-end run against a local backend, Samples & Data -> import. The chain completes and ANARCI executes. Four things only the run could find: 1. .unique() broke dedup. Polars unique() made the pt plan non-reproducible, so the body's two renders disagreed and the run died with a CIDConflictError on a field of the shared upstream node — pointing at the file both consumers read rather than at the body that produced two different graphs. Removed; it was collapsing whole-row repeats, which the identity-uniqueness gate has to do properly anyway. 2. Polars and execs cannot share one processColumn body. A body is re-rendered once per declared output and must therefore be reproducible; pt.workflow() is and exec.builder() is not. Split into two passes — polars in the first, ANARCI in the second over its output — which is the structure process-bulk already uses. annotate-regions goes back to being a template, the shape it had before I converted it to a library. 3. The second pass receives the file, not a map of files. With no axes left to aggregate over, the group value is the resource itself; both shapes accepted. 4. Pre-created ANARCI placeholders are read-only. Writing anarci.csv_H.csv into the workdir so saveFile always finds it is what antibody-tcr-lead-selection does, but ANARCI then cannot overwrite it: "Permission denied". Removed, and the single-bucket case is marked as a KNOWN GAP in the template — a set whose sequences all land in one bucket will still fail on the missing save. Test drives Samples & Data through mutateBlockStorage (it is V3) and this block through setBlockArgs (V1 storage; done(2) is the UI API version, not the storage one). Fixture is five hand-authored rows of published INN reference sequences, not the customer panel, carrying a full pair, a heavy-only row, an unnumberable sequence, and two rows with identical sequences under different names. The test asserts only that the block completes. It does not yet assert the emitted columns, so the column contract remains unverified behaviour.
block-tools 2.13 and package-builder 3.15 — both current latest — disagree about
how a build is told it is a local dev build, and the structurer's own turbo.json
closes the remaining route:
- block-tools writes scripts setting PL_BUILD_CHANNEL/VARIANT/LOCATION and
never reads PL_PKG_DEV;
- package-builder reads PL_PKG_DEV and has never heard of PL_BUILD_*;
- turbo.json's build.env lists the PL_BUILD_* set only, so a hand-set
PL_PKG_DEV is stripped before pl-pkg sees it.
The result is that .sw.json always carries a registry path. For a software
package that is not published yet, the backend then 404s and leaves zero-byte
archives in its cache, and the run dies with "package ... not found in any of
endpoints" — a message that says nothing about the env plumbing that caused it.
Two lines fix it: PL_PKG_DEV in turbo.json's build.env so it survives turbo, and
PL_PKG_DEV=local in the build:dev-local and test scripts so the sanctioned
commands work with no incantation. Verified: pnpm build:dev-local now emits
local.path descriptors and pnpm test runs the whole chain green.
BOTH FILES ARE STRUCTURER-MANAGED and the next structure refresh will revert
this silently, exactly as it reverted the packageManager pin. Reported upstream
so the fix lands in the tool instead; until then, re-apply after any refresh.
The run was green while asserting only that the block completed, which proves
nothing about the column contract. It now checks the emitted specs.
Adds an importedColumns model output: one entry per exported p-column with its
axes, domain and annotations, read from the workflow's own output rather than
derived from the mapping. It reports what was produced rather than what was
intended, which is the distinction that matters when a downstream block cannot
see the dataset and the question is whether a column is missing or merely keyed
differently. It also fills a real gap — after a successful bare import the block
displayed nothing, because the bare path returns an empty stats frame.
Asserted, all against a live run:
- every column on [pl7.app/sampleId, pl7.app/variantKey]: one frame, not one
per chain;
- the key axis carries the VDJ run id and receptor, and neither the peptide nor
the amplicon run-id key, and no retired structure key;
- the amino-acid variable domain per chain, with isMainSequence and
isAssemblingFeature, and no convention key — a whole domain has no boundary;
- fourteen region columns, seven per chain, FR4 included, each carrying the
scheme in BOTH domain and annotation;
- one status per chain with the closed three-member enum;
- the synthetic abundance with exactly the triple Clustering queries plus the
anchor, and no unit;
- exactly one column claiming anchor and primary abundance;
- no sampleCount;
- the label column, so the scientist's own identifier shows rather than a
minted C-XXXXX.
Teeth verified by mutation: flipping the sampleCount assertion to expect it
present fails the run. Without that check a green test here would be worth
nothing, which is how the previous one passed for three runs while the whole
annotation branch was dead code nothing demanded.
The record key is the hash of the identity column, so a repeated identity is a
repeated key and two different antibodies merge into one record with nothing to
show it happened. On the acceptance panel that is 119 of 1,243 records — the
measurement the whole key design rests on.
The check runs in prerun, over the whole file, because the answer decides whether
the run may start rather than describing a run that already happened. The line it
draws is not "identity repeats":
same identity, any other mapped cell differing -> refused
identical in every mapped cell -> fine, the rows state the same
record twice and collapse
so whole-row duplicates are removed first and only then are identities counted.
Read straight from prerun in argsValid, NOT mirrored into uiState from a UI
watcher. That mirror is what the format-validity flags do and it is a hairpin:
an output written back into state that a derivation then reads. It survives on
one client and races on two. An absent answer counts as invalid — not knowing is
not the same as knowing it is fine.
Also restores the whole-row collapse the CID-conflict fix had to remove, now with
keep and maintainOrder pinned. The default keep is order-dependent, which is what
made the polars plan non-reproducible in the first place.
VERIFIED GAP, found by asserting it rather than assuming: argsValid disables Run
in the interface but the platform does not enforce it — project.runBlock resolves
on an invalid block. "The run does not start" therefore holds for a scientist
clicking Run and not for an API caller. Closing it needs a workflow-side refusal,
which is data-dependent and so needs its own awaiting template. The test pins the
current behaviour so the day it changes is visible.
Second fixture and test: AB-001 twice with different light chains must collide,
AB-002 twice identically must not.
ANARCI writes one CSV per bucket it actually found, so a heavy-only set leaves no _KL.csv. The step has to declare up front which files it will save, and saving one the run never wrote fails it — which is why a heavy-only set could not be imported at all. Pre-creating placeholders is what antibody-tcr-lead-selection does and it does not work here: a file the builder writes into the workdir is read-only, and ANARCI dies with "Permission denied: anarci.csv_H.csv" trying to overwrite it. So the FASTA always carries one reference domain per bucket — trastuzumab VH and VL, published sequences numberable under every scheme. Both CSVs then exist whatever the scientist's set contains. Their ids name buckets rather than chains, so parse_fasta_id rejects them for free and nothing downstream has to know they were there; the per-chain tallies are built from the input rows, not from ANARCI's, so the counts are unaffected too. Cost is two extra sequences numbered per run. Third integration test covers it, and asserts the negative as well: only the mapped chain is emitted, with no empty B columns invented for a chain the file never had. Padding verified load-bearing by mutation — disabling it fails the heavy-only test and only that one.
…undance The contract changed under us. bare-set-synthetic-abundance-column went 1.0.0 -> 2.1.0 and replaced a column minted for this spec with the block's existing one: pl7.app/vdj/clonotypePresence, Int, no unit, "Present in Sample", order 90000 -> pl7.app/vdj/uniqueMoleculeCount, Long, unit molecules, "Number of UMIs", 88000 The spec is now copied whole from infer-columns-common.lib.tengo:50-66, with isPrimary and isAnchor stamped as the block already stamps them on whichever count is primary. The column introduces no vocabulary, no new value type and nothing for a consumer to have been taught; the fabrication is confined to the value. Note what reversed: the previous atom said no abundance/unit may ever be set, because a unit would be a false claim about molecules nobody counted. Adopting the block's spec brings the unit and the label with it, and the atom now states the cost plainly — a panel that was never sequenced is presented as "Number of UMIs" in every consuming block, and Clustering reads the name off this spec and carries molecule-named aggregates into its own outputs. The fabrication is confined to the value; what the value is called is not. The emitted literal is now cast Int64 to match the Long the adopted spec declares. Tengo unit tests and the integration assertion both re-pinned to the new contract.
Closes the last unimplemented piece of the import contract. A column holding anything the closed canonical vocabulary never anticipated had no slot to be given, however ordinary the value was, and was dropped twice over before the table was written. Each one the scientist accepts is now emitted on [sampleId, variantKey] as pl7.app/vdj/importedProperty/<sanitized header>. The name carries the header sanitized and the label carries it verbatim, which is both workspace precedents' shape rather than a choice made here: a raw header in a name would put spaces, slashes and unicode into an identifier every downstream query matches on. Two headers that sanitize to the same token are refused, naming both. Their specs would otherwise be identical — same name, same axes, empty domain — and dedupe into one column, losing a column the scientist explicitly chose. A generated suffix would keep the run going and leave names matching nothing in their file. valueType comes from the mapping and is emitted unchanged; nothing converts between types and nothing re-reads the values. Detection is not built yet, so the scientist picks it and it defaults to text — which the contract allows, since it is the type accepted at mapping that gets emitted. Detection will pre-fill this rather than change it. SPEC ERROR FOUND: the contract atom cites `Target (nM)` and `Target-nM` as a colliding pair. Under the sanitizer it names they are not — a trailing `)` leaves a trailing `_`, giving `Target_nM_` and `Target_nM`. The mechanism is right, the example is wrong. Both behaviours are pinned by tests, including the non-collision, so nobody "fixes" the code to match the prose.
The block gains a second door. The scientist either selects an already-loaded dataset, as before, or points it at a file; both are offered and exactly one may be set. Replacing the pool door was rejected upstream — it would break projects that already import custom sets, and cost the scientist whose table arrived alongside other data or who wants the sample metadata samples-and-data manages. Two doors, one emitted shape. On the direct door the block builds the same thing the pool would have handed it — a File column on pl7.app/sampleId with one entry — so everything downstream is identical and nothing can tell which door was used. The integration test asserts exactly that, with no Samples & Data in the project at all. Import mints the sample itself, one per file, labelled with the filename stem: what samples-and-data would have produced, which is what makes the doors agree rather than merely coexist. The id is minted at the user's gesture rather than derived from the handle, so the sample keeps its identity across runs. The block also emits the pl7.app/label column for it — on the pool door samples-and-data emits that, and on this one nobody else will, so without it the sample renders as an opaque id in every table downstream. Uploads are driven by isActive outputs on both the main and prerun sides. getImportProgress is what starts the transfer; without the flag the output never renders, the upload never begins, prerun never resolves, and nothing errors. The delimiter is read from the file's first line rather than taken from the extension. Prerun imports the file and supplies the header columns, which on this door nothing upstream provides. SCOPED OUT, deliberately: xlsx. The atom accepts csv, tsv and xlsx with the workbook converted at the UI boundary, and the precedent block does that by vendoring a local xlsx tarball. That is a dependency decision that does not belong in this step, and csv/tsv is the pipeline contract either way — the acceptance panel reaches it as tsv. The file input accepts csv, tsv and txt; xlsx is additive later. Also fixes an eager wf.resolve at the top of the body, which asserted on the direct door where there is no ref to resolve.
… stops Phase 5's first consumer, and the first execution of its antibody_tcr_universal branch — the one branch in the workspace that reads a bare set correctly, which no producer had ever reached and whose own test suite is entirely it.todo. Two things it proves and one it blocks on. Proved: the set is offered in the picker without the scientist inventing a column, and the modality is detected correctly. detectMode reads pl7.app/variantKey plus pl7.app/vdj/clonotypingRunId and routes to antibody_tcr_universal, which is exactly what the run-id key exists to do. The anchor arrives with the axes and domains the contract specifies. Blocked: the run then panics with "antibody/TCR mode detected but no amino-acid VDJ sequence columns found". The consumer's bundle query does not match property columns that carry the sample axis. Every working producer emits them on the record axis alone — including this block's own per-chain path, which reaches that shape through a second processColumn pass that aggregates pl7.app/sampleId away before writing the property columns. bare-set-key-axis mandates the opposite: every emitted property column on [pl7.app/sampleId, pl7.app/variantKey]. So the contract and the consumers disagree, and sequence-properties-needs-no-change is falsified — something has to move, and which is a spec decision rather than an implementation one. Not guessed at: partialAxesMatch defaults to true in the column-selector spec, so a two-axis column ought to match on that reading. The empirical result says otherwise and the working-producer shape agrees with the empirical result, so the mechanism is not fully established — only the outcome. The chain test is committed and skipped, with the finding in a comment rather than a bug title, so it is re-enabled by settling the contract rather than by remembering.
Un-skips the chain test now that the consumer accepts a dataset whose property columns keep the sample axis. The antibody_tcr_universal branch runs for the first time in its existence and emits 30 columns from a bare paired set. The test resolves sequence-properties from the local checkout via a link: dep, because the fix it needs is unreleased. Point that back at a published version once it ships — it is the one thing in this suite that will not work on a clean machine.
A spreadsheet is a boundary format — the pipeline speaks csv/tsv and nothing in the platform ingests a workbook as data. The first worksheet is now converted to csv at the point the file enters, so everything downstream, the dataset's fileExtension domain included, only ever sees csv or tsv. Converted in the workflow, not the browser. The atom describes a UI-boundary conversion and cites immune-assay-data, but that block converts workflow-side too (software/xlsx-to-csv, invoked at workflow/src/analysis.tpl.tengo:28-38); its UI-side xlsx reader only builds a column list for its mapping panel. Following what the precedent does rather than what the atom says it does buys two things: no vendored JavaScript xlsx dependency — both precedents get the library from a local tarball in a vendor directory this workspace does not have — and one converted file that the header list, the identity check and the import all read, instead of the browser parsing the original and the pipeline parsing it again. Worth raising against the atom: as written it points an implementer at the harder half of the shipped precedent. software/xlsx-to-csv is its own package rather than a third entrypoint on region-annotation, so openpyxl stays out of the ANARCI image. Converted once per workbook despite prerun and the main run both needing it, because the exec is cached. Fixture is the same panel as .xlsx with a title line above the header, which is what workbooks actually look like and what the converter's header-row heuristic is for. The test asserts the emitted shape matches the tsv of the same panel, so the title row was skipped and the real header was used. Carried over from the precedent, deliberately: first worksheet only. A workbook with its data on a later sheet converts to the wrong thing silently. The precedent has the same limitation and the same TODO.
All four were invisible to the test suite and all four were found in minutes with the block open in the app. 1. Loading a file was a dead end. The file input sits above the format gate, so a scientist loaded a workbook and got no mapping controls — those are gated on format === custom — and a disabled Run with nothing saying why. The direct door serves the custom format and no other, so picking a file now settles the format too, and the format dropdown is hidden while a file is loaded: choosing an instrument format there would silently not work. 2. argsValid THREW rather than returning false. Reading ctx.prerun from the inputsValid callback fails with "Error in block model inputsValid" — the callback does not get the prerun accessor that output lambdas do, so the identity-uniqueness gate left Run permanently disabled on a set with no collisions at all. Earlier I verified RenderCtxLegacy has a prerun getter; that was the wrong thing to verify, since having the getter does not mean the runtime supplies its accessor here. The refusal is removed from argsValid. The collision set is still computed and still shown, but the refusal that actually protects the data now has to live in the workflow, and until it does a colliding set can be run and will merge records. That is a regression against identity-uniqueness-gate, recorded rather than hidden. 3. fileImports errored with "getProgressId: wrong resource type: std/map". The main side wrapped the upload handle in a map while the model calls getImportProgress on the field directly. Prerun already returned a single resource, which is why only one side was broken. Both now return one resource, which is also honest — the direct door takes exactly one file. 4. Workbooks with a trailing empty column produced an empty header, offered as a blank option in every mapping dropdown and mappable to nothing. Filtered. Also drops a doubled pl7.app/trace on the region columns: both processColumn passes injected the same import step, and the second inherits it from the first. And removes a makeTrace that was computed and never used. Verified on the real 1,243-row Paired_INN workbook: Done, every output ok, 20 columns with the specs the contract fixes — Heavy/Light labels, seven regions per chain with imgt in both domain and annotation, both main sequences carrying isMainSequence, both status columns with the closed enum.
…table A successful import displayed nothing. The block's table is bound to `stats`, the other formats fill it with per-chain import statistics, and the bare path returned an empty frame — a gap I had noted and not closed. A bare set has no statistics in the usual sense: no reads, no UMIs, and its cross-sample aggregates are deliberately not emitted. What it does have is the outcome of the annotation, which the numbering step already computed and which I dropped when reshaping that template into a processColumn body. Restored, and shown: per chain, how many records were annotated, how many the input could not support an answer for, how many the instrument could not resolve, and how many it numbered as the other chain. Showing the imported records themselves was the first attempt and it does not work: createPlDataTableV2 fails on these Parquet-backed columns with "Key not found ctl/file/blobInfo". The block has never rendered its own dataset — only stats — so that path is unproven, and the statistics are the more useful thing to show anyway. Two schema traps on the way, both silent until the run: TsvContent validates its axes and columns against narrower schemas than the Xsv ones. Axes take only column and spec — no naRegex. Columns take only column, id and spec — no allowNA. Both are copied by habit from the Xsv shapes next to them. On the real 1,243-row Paired_INN panel the table now reads: Chain A (Heavy) 1243 annotated, 0 failed, 0 not applicable, 0 disagreed Chain B (Light) 1243 annotated, 0 failed, 0 not applicable, 0 disagreed Every record annotated on both chains, and zero chain disagreements — which is the measurement the declared-versus-inferred reconciliation question was deferred for want of. On this panel the answer is nil.
…abel convention Two defects in one column, both reported from a real project. The label sat on [sampleId, variantKey]. A pl7.app/label column only acts as an axis's label when it sits on that axis ALONE; on two axes it is merely a per-record property and the axis goes on displaying the opaque hash — which is the entire thing record-label-is-the-identifier exists to prevent. Both precedents agree: samples-and-data puts its sample label on [sampleId], and the per-chain path's own clonotypeLabel lands on [clonotypeKey] because its pass aggregates the sample axis away. So the label now comes from a pass that does the same. It cannot come from the per-sample import, whose outputs all inherit the sample axis from the partition. Collapsing is lossless: the identity is unique per record and identical in every sample it appears in. And its annotations were inconsistent with the Sample Name column beside it. The workspace convention for a column that labels an axis is the pl7.app/label name, a display name, and pl7.app/isLabel — samples-and-data, differential-expression and differential-clonotype-abundance all emit exactly that. The record label had no isLabel, and carried a visibility and an order priority, which make it a table column of its own showing the same text a second time beside the axis it is meant to be naming. It now matches. Deliberately NOT copying the block's existing clonotypeLabel here: that one omits isLabel and sets visibility and order, so it is the inconsistent one. Both rules are now pinned. A tengo test asserts the convention, and the integration assertion that every property column carries [sampleId, variantKey] excludes the label explicitly and asserts its single axis instead — so neither half can regress into the other. Verified on the real panel: label on [pl7.app/variantKey], isLabel true, no errors.
The panel offered both doors at once, separated by an "or select a loaded dataset"
rule, and mixed their controls: the format dropdown belonged to one, the file input
to the other, and the mapping slots sat under a format gate that the file door had
to force. A separator is a section heading, not a disjunction.
Now a "Load from file" checkbox at the top chooses the door, and only that door's
controls are shown:
on file input, then the columns to import — identity, heavy and light chain,
numbering scheme, and any non-sequence columns to carry through
off select dataset, data format, chains, and the existing per-format mapping,
exactly as before
Switching clears the other door, so exactly one is ever set and the "exactly one
door" rule in argsValid cannot be violated from the interface.
The toggle is ui state, not args: the block already knows which door is in use from
whether fileSource or datasetRef is set and derives nothing from the flag. It exists
so the panel can show one door's controls before either is filled in. It falls back
to whichever door is actually in use, because V1 ui state does not backfill new
defaults into blocks that already exist — without that, a block created before this
change shows the dataset door while holding a loaded file.
Also folds a nine-deep nested ternary that mapped format ids to display names into a
lookup over the same options the dropdown is built from; it had already drifted,
missing the airr-sc case in one of its two copies.
SCOPE NOTE: the bare-sequence mapping is now only on the file door. The workflow
still accepts a bare set through the pool door — the tests drive exactly that — but
it can no longer be configured that way from the interface. That follows the ask;
re-exposing it is moving one template block.
Switching to the file door and back left the dataset door showing the entire per-format mapping — CDR3 aa, CDR3 nt, V gene, J gene, count type, and the optional accordions — all empty and all required, under a "Custom" format nobody had chosen and with no dataset selected. Two causes, both mine. The interface was setting format = "custom" when the file door was used, because argsValid reached the bare-set branch only under that format. The value then outlived the door. Fixed at the root: the DOOR decides which validation applies. If fileSource is set, the bare-set mapping is what gets validated, whatever the format field says — so the interface no longer writes a format the scientist never picked. That also matches the atom: the direct door serves the custom format and no other, which is a property of the door rather than something to record in state. And switching back now clears the format, so the dataset door opens on a clean choice rather than inheriting one — including for any block still carrying a "custom" written by the previous build. customMapping is deliberately kept: re-pick a format and the scientist's own mapping is still there. The validity rule now lives in one place, bareSetValid, exported from the model and used by both argsValid and the panel's own completeness check, which had drifted into two nearly-identical expressions. The panel's stay-open rule tested format === custom, which stopped meaning anything on the file door once it no longer sets one.
A block with no dataset and no file reached wf.resolve with an undefined ref and failed the assertion. The failure was not contained: the block's exports are in the result pool, so a downstream block scanning the pool inherited it and reported an error naming import-vdj's main template — which reads as a bug in a block the scientist was not touching. That is how it surfaced: Sequence Properties failed because a half-configured sibling Import block existed in the same project. Now it returns empty outputs and exports in that state, as prerun already did.
A bare set now carries MiXCR's shape: the abundance column keeps [pl7.app/sampleId, pl7.app/variantKey] and everything else — sequences, regions, statuses, imported properties, label — sits on [pl7.app/variantKey] alone. MiXCR does the same thing in aggregate-by-clonotype-key.tpl.tengo, grouping clonotype properties onto the key and leaving only abundance per sample. The old shape put every column on both axes. Consumers match axes positionally — a selector naming one axis is compared against the candidate column's axis 0 (BQueryAxesKeys.Accept, core/pl controllers/shared/resources/block_resolver_request.go:244) — so a property column with the sample axis in front matched nothing, and sequence-properties failed with 'no amino-acid VDJ sequence columns found'. The fix belongs here, in the producer: widening each consumer's selector instead would have meant touching sequence-properties, antibody-sequence-liabilities, clonotype-space and clonotype-clustering, each with its own blast radius. process-bare-set now runs four passes. Import and ANARCI stay per sample, as in MiXCR; two aggregate passes collapse their TSVs onto the record key. Both share one body, aggregate-by-record, which selects nothing — the caller's Xsv settings decide which columns are read — so it replaces the single-purpose aggregate-record-labels. Collapsing is lossless: every collapsed column is a function of the record key and identical in every sample the record appears in. columnsBySource returns perSample / perRecord / regions instead of records / regions. Two tests pin the split: that abundance is the only per-sample column, and that the three buckets partition allColumns exactly. A column silently dropped from the split would never be emitted, with nothing to report it. Also: the direct door's sample label column read 'Sample Name'; samples-and-data uses 'Sample'. The atoms fixing the old shape (630, 650, 670, 690, 695) are being updated separately.
…ce-properties The two integration assertions pinned the old shape and failed as intended; they now require abundance on [sampleId, variantKey] and every other column on the record axis. test/package.json pointed sequence-properties at the local checkout, because the consumer needed a partialAxesMatch widening to read a bare set. It no longer does — the producer carries the right shape — so the dep goes back to the published 1.5.2 via the catalog. A test linked against a local checkout could not distinguish 'consumers need no change' from 'the change is sitting unmerged next door', which is the whole claim this test exists to make. 6/6 green against published sequence-properties 1.5.2.
…lots Five UI changes to the bare-set mapping, plus the two workflow changes two of them need. Title and dataset name. The file door left defaultBlockLabel to the chain list, so every such block was titled 'IGHeavy, IGLight, TCRAlpha, TCRBeta, TCRDelta, TCRGamma' — six defaults the scientist never chose. It now shows the file's name, and the numbering scheme after it when that is not IMGT. The trace step label follows: it was the constant 'Import V(D)J Data', which is what a downstream dataset dropdown shows, so two imports were indistinguishable there. MiXCR labels its step 'MiXCR ' + preset name for the same reason. Sequence slots. The chain dropdowns offered every header, so 'mAb ID' could be mapped as a variable domain. Nothing refuses it downstream — ANARCI simply fails to number it and every record comes back Failed, which reads as a data problem rather than a mapping one. A header cannot tell an identifier from a domain, so prerun now reads 21 lines instead of 1 and classifies each column by alphabet and length: >=80% of sampled values matching the 20 residues plus X and *, and at least 50 characters. The threshold is what keeps antibody INNs out — trastuzumab and adalimumab are alphabetic but carry u/z/b and are far too short. Empty classification means 'not determined' and the UI falls back to every header, because an empty dropdown is a worse failure than an unfiltered one. Layout. 'Other columns' is gone and record properties sit under the light chain, in the same section as the sequences they sit beside in the file. The numbering scheme moved below into a 'Region annotation' section: it maps no column, and listing it among the mappings read as one more thing to assign. The integration test pins the classification against the fixture, whose headers are mAb ID / VH / VL / Affinity (nM): the answer must be exactly [VH, VL]. 6/6 green.
V1's args/uiState split becomes one BlockData, with .upgradeLegacy carrying every
field across for projects already on disk.
The channel decision, field by field, is the substance — a straight copy into
BlockData with an identity args lambda would be V1 in a V3 jacket:
data only defaultBlockLabel, customBlockLabel, secondaryCountType,
tableState, settingsOpen, loadFromFile, the five per-format
presence flags
args datasetRef, fileSource, format, chains, customMapping,
primaryCountType, bareSet
prerunArgs the same minus chains, which prerun never reads
The labels are the point. Under V1 they sat in args, so renaming a block changed
args and asked the scientist to re-import. The workflow reads neither
(grep args. over main.tpl and prerun.tpl), and secondaryCountType likewise —
it decides which count columns the panel offers and reaches the workflow only
through the customMapping it produces.
The args lambda also strips: the door that is not in use, and the dataset-door
mapping fields once a bareSet is configured, since the bare branch of the
workflow reads only bareSet/fileSource/datasetRef. It deliberately does NOT sort
chains — canonicalising a set the user picked in their own order would change
args for every project on disk to buy the rare re-pick-in-another-order case.
argsValid becomes throws inside the args lambda. That is a clean port here
because the V1 validator never touched ctx.resultPool — it read ctx.args and
ctx.uiState only, both of which are now data. No snapshot pattern needed.
Two fields leaving args means every existing block goes stale once. The re-run
is cheap: the workflow's pure templates key on their own inputs, which have not
changed.
Tests: setBlockArgs hardcodes modelAPIVersion 1 and throws on a V3 block, so the
six integration tests drive the block through mutateBlockStorage. A blockData()
helper fills the view-state half, because update-block-data replaces data
wholesale rather than merging.
upgradeLegacyData is extracted as a pure function and unit-tested, because the
upgrade is unreachable from the integration suite: fresh projects exercise
.init(), and a V1 project cannot be built against a V3 block to drive the
upgrade. A missed field there is silent — it reads as a setting the scientist
lost when reopening an old project. 5 cases pinned.
11/11 green.
The panel asked for Text / Whole number / Decimal per accepted property column, and that question could not be answered reliably by the person being asked. Nothing re-reads the values: the declared type goes straight onto the column spec, so a column typed Decimal that holds 'N/A' on row 500 fails at import or nulls out, far from the dropdown where the choice was made. Sampling the file to guess instead was available — split-header already reads 21 lines for the amino-acid detection, so the classifier would have been a few lines in a pass that already runs. Not taken, because it has the same tail in a quieter form: the guess comes from the first rows and is applied to all of them, and a column that is numeric for 20 rows and textual at row 500 would be typed from what was sampled. A string is the one answer that is never wrong, and a downstream block that needs a number can convert a column it can see in full. ImportedProperty loses valueType and is now just a header. Projects that already carry one keep the field on disk harmlessly — nothing reads it — and their property columns become String on the next run. The bare-set path is unreleased, so no shipped dataset changes shape. 11/11 green.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # thing silently, which is worth a picker eventually — the precedent carries the same TODO. | ||
| ws = wb[wb.sheetnames[0]] | ||
|
|
||
| rows = list(ws.iter_rows(values_only=True)) |
There was a problem hiding this comment.
list(ws.iter_rows(...)) retains the entire expanded worksheet despite opening the workbook in read-only mode. For large user-supplied workbooks, this makes conversion unnecessarily slow and memory-intensive and can exhaust the converter worker's 16 GiB allocation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: software/xlsx-to-csv/src/main.py
Line: 39
Comment:
**Eager worksheet materialization**
`list(ws.iter_rows(...))` retains the entire expanded worksheet despite opening the workbook in read-only mode. For large user-supplied workbooks, this makes conversion unnecessarily slow and memory-intensive and can exhaust the converter worker's 16 GiB allocation.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
pl7.app/vdj/scClonotypeChain has become the marker consumers use to recognise a dataset holding paired chains in one frame: clonotype-clustering probes for it and antibody-sequence-liabilities scans for it, both added in this work. A set with one mapped chain is bulk-shaped, so stamping it there made those blocks treat a one-chain import as paired — the marker claimed a shape the data did not have. columnsBySource and allColumns derive paired := len(chains) > 1 and pass it down, so the process template cannot get it wrong. With one chain the main sequence, the seven region columns and the status carry no scClonotypeChain and no /index; with two they are unchanged. Atom 630 justifies the key by distinguishing the two columns from each other, and that reason genuinely evaporates with one chain. Atom 650 gives a second one — that sequence-properties reads the key and defaults it to A when absent — which looked decisive until checked: chainNames() maps chain A to VH/CDR-H3 and anything else to VL/CDR-L3, so a bulk MiXCR light-chain dataset is ALREADY labelled VH today. A light-only import now behaves the same way. That is the bulk convention, inherited rather than introduced, and the chain is still named in every column's label. Two tengo tests pin the rule: that a one-chain set carries no chain key while keeping feature, alphabet, scheme and label, and that columnsBySource stamps all 18 paired columns and none of the single-chain ones. The stats table's axis is still pl7.app/vdj/scClonotypeChain. It is not a pl7.app/vdj/sequence column, so neither consumer's probe sees it, and it remains a per-chain report. It is now the only place the raw A/B letter surfaces.
The Chain axis of the annotation statistics table displayed the raw A. Every data column already reads 'Heavy ...' through CHAIN_LABELS, but the stats axis takes its values from the python, which writes the declared chain letter, and nothing labelled them. A pl7.app/label column on the chain axis now carries Heavy / Light — the same split the record axis already uses, where the value is an opaque hash and a label column holds the identifier. The axis values stay A / B rather than being rewritten to Heavy / Light in the python. pl7.app/vdj/scClonotypeChain is a shared vocabulary and every other producer puts A / B in it, so an axis whose values disagreed with the same-named column domain would be a second meaning for one name. Built in the workflow rather than added to the per-sample statistics TSV because a pl7.app/label column only names an axis when it sits on that axis alone. Read from the TSV it would land on [pl7.app/sampleId, chain] and be just another property — the same mistake the record label made before it was moved to its own aggregate pass. The heavy-only integration test asserted the chain domain was present on a single-chain set, which the previous commit removed. It now asserts the opposite, plus that the eight sequence columns still carry the chain in their labels. 11/11 green.
The statistics table keyed rows on a pl7.app/vdj/scClonotypeChain axis, so it displayed a Chain column of raw A / B letters. The block's bulk path models the same information the other way round — chain in each stat column's domain, no chain axis, chain in the label (process-utils.lib.tengo buildStatsColumns, axes: [] at process-bulk.tpl.tengo:183) — and so do the bare set's own region columns. One block, two shapes for one thing. The bare path now follows bulk. statsColumns takes the mapped chains and emits four statistics per chain on [pl7.app/sampleId] alone: 'Heavy Annotated', 'Light Annotated' and so on. With one chain the domain key and the label prefix are both omitted, so a single-chain import's statistics are shaped exactly like a bulk import's — which is the consistency the previous commit established for the data columns and left unfinished here. The stats TSV goes from long to wide: one row with <chain>_<statistic> columns instead of a chain column with a row each. That is the shape a TsvContent output with no axes reads. This replaces the chain-label column added in the previous commit, which named an axis that no longer exists. Labelling the axis treated the symptom; the axis was the problem. statsAxis and chainLabelColumn are both gone. Tests: a tengo test pins the paired and single-chain shapes and that the two chains' columns stay distinct specs; the region-annotation suite's stats assertion follows the TSV to its wide form. 11/11 integration + 6 tengo + the python suite green.
Completes the bulk alignment. With one mapped chain the record is that chain, so the variantKey axis carries pl7.app/vdj/chain — IGHeavy or IGLight — the same key and the same vocabulary the bulk path puts on its clonotypeKey axis (process-bulk.tpl.tengo:128). A paired record holds both chains, so no single value applies and the key is absent; chain is a property of the column there. This matters more than tidiness now. The columns of a single-chain set stopped carrying scClonotypeChain two commits ago, so without this nothing machine-readable said which chain the set was — only the labels did, and no consumer reads labels. The axis is where bulk has always put it. The vocabulary is IGHeavy/IGLight rather than the A/B slot names because that is what pl7.app/vdj/chain holds everywhere else, and sequence-properties' CHAIN_TO_RECEPTOR already maps both to IG. Receptor is stated directly as well, so this is a second, consistent route to the same answer rather than a new dependency. Three key-axis tests move to the paired form and a fourth pins the single-chain one: heavy-only names IGHeavy, light-only names IGLight, paired names nothing. 11/11 integration + 6 tengo green.
The two sequence slots were A and B everywhere — model type, UI handlers, tengo CHAINS, python chains.py, TSV column names. That is the pl7.app/vdj/scClonotypeChain vocabulary, which exists to tell apart chains sharing one frame. A single-chain import has nothing to tell apart and is a bulk shape, so naming its one slot 'A' described the wrong structure. Slots are now IGHeavy / IGLight: the pl7.app/vdj/chain vocabulary the block's bulk path already uses, which the key axis of a single-chain set now carries, which sequence-properties' CHAIN_TO_RECEPTOR already understands, and which extends to TCRAlpha/TCRBeta if bare sets ever accept them. A/B extends to nothing. Emitted specs are unchanged. pl7.app/vdj/scClonotypeChain still carries A / B on a paired set, translated at the point of emission through PAIRED_CHAIN_DOMAIN. Two vocabularies because there are two questions: which locus the scientist mapped, and which position a chain occupies in a paired record. Conflating them is what this commit undoes. Internal only, so the TSV column names move with it: IGHeavy_sequence, IGHeavy_CDR3_aa, IGHeavy_annotated. The tengo specs and the python agree on that convention and had to move together. A bare-set mapping saved before this keeps its columns under the old slot keys and will need re-selecting. The bare path is unreleased. 11/11 integration + 6 tengo + the python suite green.
ANARCI writes one CSV per bucket it actually found, so a heavy-only set leaves no
_KL.csv and saving a file the run never wrote fails the step. The workaround was to
append one reference domain per bucket to the FASTA — trastuzumab VH and VL — so
ANARCI always found both and always wrote both files.
That workaround existed because of a wrong conclusion. Pre-creating the files hit
'Permission denied: anarci.csv_H.csv', and I read that as pre-creation being
impossible. It is the default, not a limit: workdir inputs land 0o400 so the backend
can hardlink them from the archive cache, and exec.builder() takes
{ writable: true } to land 0o600 with a copy to a fresh inode instead. It is
documented on addFile and writeFile in sdk/workflow-tengo/src/exec/index.lib.tengo,
and peptide-extraction/workflow/src/parse.tpl.tengo:28 already uses exactly this
pattern for exactly this purpose. I did not look.
Both CSVs are now pre-created as writable header-only placeholders. ANARCI overwrites
the buckets it finds and leaves the others alone, so both saves always have a file.
PAD_KEY, PAD_SEQUENCES and the padding loop are gone, and no synthetic sequence enters
a scientist's pipeline.
The open question was whether ANARCI overwrites the placeholder or appends to it. On
an append the header row would be the placeholder's 'Id', no position column would
parse, and every record would come back Failed with the columns still present — which
the integration tests would not catch, since they assert the columns exist. Verified
on the real 1,243-record paired panel instead: Heavy 1243 annotated / 0 failed, Light
1243 annotated / 0 failed. It overwrites.
Two padding tests retired. The third is kept and retargeted: an id whose suffix is
not a mapped slot must never become a record. Nothing writes such an id now, but the
parser is what keeps a stray one out of the dataset. The pre-existing 'header-only
bucket tolerated' case turns out to be the assertion this change needed.
11/11 integration + 6 tengo + the python suite green.
forceSettingsOpen held the panel open whenever a custom mapping was incomplete, and onModalUpdate swallowed the close. So a scientist part-way through mapping could not look at the table, re-read their file, or check what an upstream block had produced without finishing first. Nothing needed it. The args projection already keeps Run disabled until the mapping is valid, and the Settings button reopens the panel. The refusal only removed a way out. Both doors, not just the new one. The behaviour predates this work — main refuses when format is custom and the mapping is incomplete — and this branch had extended it to the file door, which is where it was reported. Unblocking only the file door would have left the same trap on the dataset door plus an inconsistency between them. mappingComplete and its bareSetValid import go with it: the refusal was their only reader.
Annotated / Not applicable / Failed are terse, and 'Not applicable' reads as a diagnosis when it means only that the cell was empty — no length check, no alphabet check, no instrument outcome. It is also the expected value on any panel where one chain is partly filled in, so it is the value most likely to be misread as a problem. The status column now carries a pl7.app/description covering all three, and says what a Failed usually indicates: not bad data, but a column mapped to a chain slot that does not hold amino-acid variable domains. The four statistics carry one each for the same reason — their labels repeat the same vocabulary, so a reader hovering either place gets the same answer. chainDisagreed also states what it does not do: the regions are recorded under the mapped chain regardless, and nearly-all-records-on-both-chains means the two sequence columns were mapped the wrong way round. Two tests: that the status description explains each of the three values by name, and that every statistic is described. Naming left alone. The vocabulary is fixed verbatim by atom 670 and lands in pl7.app/discreteValues, where it is a filter chip the scientist sees, so renaming 'Not applicable' to something like 'Not supplied' is a spec change rather than an implementation one.
Record properties were String because nothing could safely say otherwise: the panel had asked the scientist for a type, which asked them to declare something nothing verifies, and the sampled alternative I proposed had the same tail in a quieter form. samples-and-data already solves this properly (ui/src/dataimport.ts:85-101): scan every row, and take the running max over an ordered type lattice — None < Long < Double < String — so the type only widens. One non-numeric value anywhere settles the column, and empty cells decide nothing. That is not a sampling heuristic; it is exact for the file it read. New software/column-profile streams the file once with the stdlib csv reader and answers both questions the panel has: the type of every column, and which columns hold amino-acid variable domains. prerun runs it over the whole file in place of the 21-line txt.head, so the alphabet detection stops being a 20-row sample too. The type reaches the workflow by the snapshot-on-gesture route rather than a watcher: the model exposes columnProfile, and the UI writes the detected type when the scientist accepts a column. The args projection stays a pure function of data, which a watcher mirroring an output back into data would have broken. Verified on a fixture built for the case that motivated it: count 5, 7, N/A -> String the late N/A settles it affinity 1.5, 2, 3.25 -> Double the int widened VH domains -> String, and flagged amino-acid The integration tests passed before this commit's test changes and covered none of it: they write block data directly, so properties arrived with no valueType, the workflow defaulted to String, and the assertion agreed. They now assert the profile prerun produced — Affinity (nM) is Double, the identity and the domains are text — and that a property carrying a detected type is emitted with it. Includes the two renames: the record axis and its label column read 'Variant Id', and the identity dropdown reads 'Select id column'. 11/11 integration + 6 tengo green.
… the list The panel asked two questions where there is one. A checkbox decided which door was showing and a dropdown then chose within it, so the scientist had to know they were on the right door before the control in front of them meant anything. One dropdown now. Pool datasets and a 'Load from file…' entry share the list; picking the entry opens PlFileDialog, the platform's own file browser and the same one PlFileInput opens. Not lsDriver.showOpenSingleFileDialog, which opens the operating system's picker and can only see the local disk — PlFileDialog lists every storage the scientist has, so a file on S3 loads the same way as one on the desktop. A loaded file appears in the list as the selected entry, so the control always shows what the block is reading rather than going blank on the file door, and re-selecting it reopens the dialog, which is how a file is swapped. Values are strings, not PlRefs, because the list mixes two kinds of entry: a canonicalised ref per dataset and one sentinel that performs an action rather than selecting anything. PlDropdownRef cannot express that — its value is a PlRef — so this is a plain PlDropdown mapping back to refs on the way out. Cancelling needs no restore: it emits nothing, and nothing is cleared on the way in, so the previous selection simply stands. loadFromFile leaves BlockData. Which door is showing is derived from whether a file is loaded, so a stored flag could only disagree with the data — and it had already needed a fallback for blocks predating it. The legacy upgrade drops it rather than carrying it, and the test that pinned the old defaulting now pins that a fileSource crosses without one. Adds canonicalize to the ui package and the catalog, matching the version four other blocks pin. 10/10 integration + 6 tengo green.
The panel had a fixed pair of chain dropdowns and read 'paired or single-chain' off
how many were filled. Those two cases emit different shapes — the chain domain on
the columns, the chain on the key axis, the per-chain statistics — so the block was
deciding its output from an accident of how far the scientist had got.
A 'Receptor / chain' selector now states it: IG, IG Heavy only, IG Light only. Slots
follow the selection and are labelled from it, and bareSetValid requires every slot
the declaration asks for. Declaring IG with one column mapped is an unfinished
mapping and the run is refused, rather than quietly emitting a heavy-only set the
scientist did not ask for.
Changing the selection drops columns mapped to slots the new choice does not ask
for. Keeping them would leave the block emitting a chain that was just deselected.
keyAxis derives pl7.app/vdj/receptor from the chains through CHAIN_RECEPTOR instead
of hardcoding IG, and asserts that a set does not mix receptors — the A/B domain
cannot express more than one receptor's pair. That is the seam TCR will use.
TCR is deliberately absent from the list rather than present and disabled. ANARCI
can number it: the shipped HMM library carries human_A/B/G/D and mouse_A/B/G/D, and
its allow-set is {H,K,L,A,B,G,D}. What is missing is ours — ANARCI_BUCKETS reads only
H and KL, and REGION_RANGES has no TCR tables. Worth recording for that work: ANARCI
restricts chothia, kabat, martin and wolfguy to H/K/L and raises
'Unimplemented numbering scheme' otherwise (anarci.py:558-592), so TCR is IMGT-only
and the scheme dropdown will have to narrow with the receptor.
10/10 integration + 7 tengo green.
Kabat, Chothia, Martin and Wolfguy were defined on antibody structures and ANARCI
implements them for H/K/L only, raising 'Unimplemented numbering scheme %s for chain
%s' for anything else (anarci.py:558-592). IMGT takes no chain type at all —
number_imgt(state_vector, sequence) — because the numbering is position-unified.
So the scheme a set can be numbered under is a function of the chains it declares.
SCHEMES_FOR_SELECTION states that, the dropdown reads it, and changing the
declaration resets a scheme the new chains cannot use rather than leaving one that
would fail the run instead of the mapping.
Inert today: every selection is IG and keeps all three schemes. It takes effect when
TCR chains are added, which is the point — the constraint is recorded where the TCR
work will need it rather than discovered from a failing run.
Two facts established for that work, both from the shipped ANARCI rather than
inference:
csv_output writes one file per chain type over ['H','KL','A','B','G','D'], with
_lc = {'K':'KL','L':'KL'} merging kappa and lambda (anarci.py:228-263). Those six
are the complete bucket vocabulary; ANARCI_BUCKETS currently reads two.
IMGT region boundaries do not vary by chain, which is why the existing REGION_RANGES
already has identical H and KL tables under imgt. The TCR tables are therefore the
same numbers, derived rather than sourced.
10/10 integration + 7 tengo green.
ANARCI could always number TCR: the shipped HMM library carries human_A/B/G/D and
mouse_A/B/G/D and its allow-set is {H,K,L,A,B,G,D}. The limitation was ours.
Two things were IG-shaped. ANARCI_BUCKETS read H and KL, where ANARCI writes one CSV
per chain type over ['H','KL','A','B','G','D'] (anarci.py:263, with _lc merging kappa
and lambda into KL). And REGION_RANGES had tables for those two buckets only.
Both now cover all six. The TCR boundaries are the IG boundaries, assigned from them
rather than restated: IMGT numbering does not vary by chain — number_imgt takes no
chain type at all — which is why H and KL were already identical under imgt. Only
imgt gains TCR entries, because ANARCI implements kabat and chothia for H/K/L alone
and raises otherwise, so those schemes can never be reached with a TCR bucket. The
scheme dropdown narrows accordingly, which the previous commit prepared.
The annotation step took --h_csv and --kl_csv; it now takes one flag per bucket,
generated from ANARCI_BUCKETS so the list the workflow saves and the list the script
reads cannot drift. The placeholder-and-save pair is emitted per bucket for the same
reason the placeholders exist at all: the step declares its saves before ANARCI has
decided which chains it found.
Slot A is the more diverse chain — the one recombining a D segment. IGHeavy,
TCRBeta, TCRDelta. That is MiXCR's rule, stated at process.tpl.tengo:695 and encoded
in its receptorInfos order, and this block's single-cell path already agreed with it.
sequence-properties labels slot A as Vα for TCRAB, which is the opposite; that is a
label-only defect on its side, left for a separate task and not followed here.
A set still holds one receptor's chains, because the A/B vocabulary has two slots
whose meaning comes from the receptor. keyAxis derives and asserts that.
Tests: the A/B assignment and the axis receptor pinned for all three receptors, and
a single-chain TCR set pinned to name its own locus. The python suite's flags follow
the CLI change.
10/10 integration + 9 tengo + the python suite green.
The previous commit's TCR support was verified by reading — ANARCI's HMM library, its per-chain-type CSV naming, IMGT's chain-independence, MiXCR's slot rule — and every fixture in the suite was an antibody. Nothing had run a TCR sequence through FASTA -> ANARCI -> regions, so 'a _B.csv appears and our range tables fit it' was a claim, not an observation. The fixture is four published human alpha/beta TCRs from RCSB PDB, fetched rather than written: A6 (1AO7), 1G4 (2BNR), DMF5 (3QDG) and JM22 (1OGA). Whole ectodomain chains, constant regions and expression tags included, because that is the shape a real export has and ANARCI locates the variable domain within it. Public reference structures, so nothing here is customer data. Verified in a throwaway project, since the assertions the test can make are about column specs and those would hold on a run where ANARCI numbered nothing: Alpha 4 annotated 0 failed 0 not applicable 0 disagreed Beta 4 annotated 0 failed 0 not applicable 0 disagreed chainDisagreed 0 on both chains is the part worth having. It means every sequence landed in the bucket its declared slot expected — beta in ANARCI's B, alpha in A — so the slot assignment is confirmed against sequences rather than against MiXCR's comment about it. The test asserts what it can reach: both slots emitted with 8 sequence columns each, labels Beta/Alpha, all 14 region columns stamped imgt, and a status column per chain. The counts above are recorded here because the block state does not expose them as plain data for a test to read. 11/11 integration + 9 tengo + the python suite green.
The selector had invented its own strings — 'IG (heavy + light)', 'TCR beta only' — where the platform already has words for these. Now mixcr-clonotyping's combined receptor-or-chain list verbatim (SettingsPanel.vue:288-301): IG, TCR-αβ, TCR-ɣδ, then IG Heavy, IG Light, TCR-α, TCR-β, TCR-ɣ, TCR-δ. Same control, same words, receptors before chains as there. Note that the two existing vocabularies disagree and this had to pick one. mixcr and this block's own regular-mode chain selector agree on the receptors and on IG Heavy and IG Light, but the regular mode calls the TCR chains TRA/TRB/TRD/TRG. mixcr's Greek forms won because this is the same kind of control as mixcr's — receptors and chains in one list — and because TRB reading next to TCR-αβ in a single dropdown is worse than TCR-β. The regular-mode list is untouched, so the block now shows both spellings on different doors; worth reconciling, but not by widening this change. The slot labels keep '(aa)': that is the alphabet, not a clarification of the chain.
The slots read 'Heavy chain variable domain (aa)' where the choice above them read 'IG Heavy'. Two names for one thing, in the same panel, one directly under the other. They now use the list's own words: IG Heavy, IG Light, TCR-α, TCR-β, TCR-ɣ, TCR-δ. The alphabet leaves the label rather than being restated six times. Everything a bare set takes is amino acid today, and when that stops being true it will be a control of its own — a suffix on every slot label would be the wrong place to say it, and would have to be removed again. 11/11 integration + 9 tengo green.
An earlier commit folded the statistics into one wide row with the chain in each column's domain, to match the block's bulk path. That was the wrong thing to copy: a paired set's two chains are compared against one another, and four numbers twice across a header is a worse shape for that than a row each. Back to a row per chain, with two things fixed rather than reverted. The axis is pl7.app/vdj/chain, not pl7.app/vdj/scClonotypeChain. Its value is the locus the scientist mapped — IGHeavy, TCRBeta — not the A/B position within a pair. That is the vocabulary the slots are named in and the one the key axis of a single-chain set already carries; A/B belongs to columns sharing a frame, which these rows are not. A label column on that axis carries the panel's own words, so the table reads TCR-β where the dropdown said TCR-β. On the axis alone, since a pl7.app/label column only names an axis when it sits on that axis by itself. CHAIN_DISPLAY mirrors the UI list. Verified on the four published TCRs in a throwaway project: two rows, TCR-α and TCR-β, 4 annotated each. The python test caught the format change on the way through, which is what it is for — the counts it pins were right, only their shape had moved. 11/11 integration + 9 tengo + the python suite green.
The workflow test fails on the runner before a single test runs: CompileError: WebAssembly.compile(): invalid value type 'stringref', enable with --experimental-wasm-stringref @+111 A wasm module reached through the SDK test stack is compiled at import time and uses the stringref value type, which node 20's V8 rejects. Reproduced locally: under node 20.16.0 `vitest run src/wf.test.ts` fails with exactly that error and no test executes; under 22.19.0 the same command passes. model.test.ts is unaffected either way because it never loads the wasm, which is why CI reported one suite passing and one failing to collect. Same move as clonotype-convergence made in MILAB-6650 for node 20 missing Promise.withResolvers — a different symptom of the same floor.
V2 throws when saved sorting or a saved filter names a column the current run does not emit, failing the whole output over a display preference the scientist cannot clear from the interface. Changing the receptor set changes the emitted columns, so the state is reachable by ordinary use. V3 filters those references instead, which replaces the catch-and-retry guard added for the same symptom. The rendered table is unchanged. Measured through the pframe driver against the same fixture: 2 rows and 8 columns under both, and visibleTableHandle equals fullTableHandle in both, so neither hides anything. Only the table definition hash differs, which is expected of a different def for the same content. The row count is now asserted rather than assumed — one row per mapped chain is what the chain axis exists for. The stale-sort test previously used "no-such-column", which V3 rejects in extractPObjectId as unparseable rather than filtering as absent. It now uses the id a project actually carries after the stats columns stopped being named per chain, which is the case that must be tolerated.
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.
Greptile Summary
The PR adds direct CSV/TSV/XLSX import for bare paired V(D)J sequence sets, ANARCI-based region annotation, record-level property emission, mapping UI changes, and a migration to BlockModelV3 and the refreshed SDK block structure.
Confidence Score: 4/5
The PR appears safe to merge, with a non-blocking opportunity to make XLSX conversion memory-bounded for large workbooks.
The accepted concern is limited to eager worksheet materialization during XLSX conversion; no blocking functional or security failure remains in the accepted findings.
Files Needing Attention: software/xlsx-to-csv/src/main.py
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[CSV / TSV / XLSX source] --> B{XLSX?} B -- Yes --> C[XLSX-to-CSV conversion] B -- No --> D[Normalized delimited file] C --> D D --> E[Prerun: headers, amino-acid candidates, identity collisions] E --> F[Bare-set mapping] F --> G[Import and record-key generation] G --> H[ANARCI region annotation] H --> I[Aggregate record-level columns] I --> J[Imported VDJ dataset]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "MILAB-6720: import record properties as ..." | Re-trigger Greptile
Context used: